checkout: also chmod in the user checkout case

When falling back to copying, we previously would only chmod checked out
files in the non-user-checkout mode. Fix this by always doing chmod.
The file_mode was being prepared but never actually applied.

Add a basic test in the archive-z2 --> usermode checkout case in which
we're guaranteed to always fall back to copy mode.

Closes: #633
Closes: #903
Approved by: cgwalters

diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c
index 8dbe49e3a3bb69dbd0e3f5cbf84daecf4b997ecb..bb7c17718243ff3280bd56d4f0fdb7fe5cfc9878 100644 (file)
--- a/src/libostree/ostree-repo-checkout.c
+++ b/src/libostree/ostree-repo-checkout.c
@@ -140,10 +140,7 @@ write_regular_file_content (OstreeRepo            *self,
     {
       if (TEMP_FAILURE_RETRY (fchown (outfd, g_file_info_get_attribute_uint32 (file_info, "unix::uid"),
                                       g_file_info_get_attribute_uint32 (file_info, "unix::gid"))) < 0)
-        return glnx_throw_errno (error);
-
-      if (TEMP_FAILURE_RETRY (fchmod (outfd, g_file_info_get_attribute_uint32 (file_info, "unix::mode"))) < 0)
-        return glnx_throw_errno (error);
+        return glnx_throw_errno_prefix (error, "fchown");
 
       if (xattrs)
         {
@@ -152,10 +149,19 @@ write_regular_file_content (OstreeRepo            *self,
         }
     }
 
+  guint32 file_mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode");
+
+  /* Don't make setuid files on checkout when we're doing --user */
+  if (mode == OSTREE_REPO_CHECKOUT_MODE_USER)
+    file_mode &= ~(S_ISUID|S_ISGID);
+
+  if (TEMP_FAILURE_RETRY (fchmod (outfd, file_mode)) < 0)
+    return glnx_throw_errno_prefix (error, "fchmod");
+
   if (fsync_is_enabled (self, options))
     {
       if (fsync (outfd) == -1)
-        return glnx_throw_errno (error);
+        return glnx_throw_errno_prefix (error, "fsync");
     }
 
   if (outstream)
@@ -249,14 +255,8 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
   else if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
     {
       g_auto(OtTmpfile) tmpf = { 0, };
-      guint32 file_mode;
       GLnxLinkTmpfileReplaceMode replace_mode;
 
-      file_mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode");
-      /* Don't make setuid files on checkout when we're doing --user */
-      if (options->mode == OSTREE_REPO_CHECKOUT_MODE_USER)
-        file_mode &= ~(S_ISUID|S_ISGID);
-
       if (!ot_open_tmpfile_linkable_at (destination_dfd, ".", O_WRONLY | O_CLOEXEC,
                                         &tmpf, error))
         return FALSE;

diff --git a/tests/basic-test.sh b/tests/basic-test.sh
index b209b8390dea1f3576b265c1b81c3fddd0ed960a..a393d7fc2fe492b0464baa49d451a6e0f15b338e 100644 (file)
--- a/tests/basic-test.sh
+++ b/tests/basic-test.sh
@@ -28,7 +28,7 @@ echo "ok yaml version"
 CHECKOUT_U_ARG=""
 COMMIT_ARGS=""
 DIFF_ARGS=""
-if grep -q bare-user-only repo/config; then
+if is_bare_user_only_repo repo; then
     # In bare-user-only repos we can only represent files with uid/gid 0, no
     # xattrs and canonical permissions, so we need to commit them as such, or
     # we end up with repos that don't pass fsck
@@ -66,7 +66,7 @@ validate_checkout_basic checkout-test2
 rm checkout-test2 -rf
 # Only do these tests on bare-user/bare, not bare-user-only
 # since the latter automatically synthesizes -U if it's not passed.
-if ! grep -q bare-user-only repo/config; then
+if ! is_bare_user_only_repo repo; then
 if grep -q bare-user repo/config; then
     if $OSTREE checkout -H test2 checkout-test2 2>err.txt; then
         assert_not_reached "checkout -H worked?"
@@ -566,6 +566,13 @@ rm test2-checkout -rf
 ${CMD_PREFIX} ostree --repo=repo2 checkout -U test2 test2-checkout
 assert_file_has_content test2-checkout/baz/cow moo
 assert_has_dir repo2/uncompressed-objects-cache
+# we're in archive mode, but the repo we pull-local from might be
+# bare-user-only, in which case, we skip these checks since bare-user-only
+# doesn't store permission bits
+if ! is_bare_user_only_repo repo; then
+    assert_file_has_mode test2-checkout/baz/cowro 600
+    assert_file_has_mode test2-checkout/baz/deeper/ohyeahx 755
+fi
 echo "ok disable cache checkout"
 
 cd ${test_tmpdir}

diff --git a/tests/libtest-core.sh b/tests/libtest-core.sh
index 14d9cd5c5793610a4eeced8a16d548c4c27c55a5..de85054475846dbd4117305ef6aebc8bce1f6df4 100644 (file)
--- a/tests/libtest-core.sh
+++ b/tests/libtest-core.sh
@@ -110,6 +110,13 @@ assert_file_has_content_literal () {
     fi
 }
 
+assert_file_has_mode () {
+    mode=$(stat -c '%a' $1)
+    if [ "$mode" != "$2" ]; then
+        fatal "File '$1' has wrong mode: expected $2, but got $mode"
+    fi
+}
+
 assert_symlink_has_content () {
     if ! test -L "$1"; then
         fatal "File '$1' is not a symbolic link"

diff --git a/tests/libtest.sh b/tests/libtest.sh
index 1a81c755a52d307f39a34aed517f2807ecc41151..f1fba4fc201a7f56f2a2119f67d38d161ab785e1 100755 (executable)
--- a/tests/libtest.sh
+++ b/tests/libtest.sh
@@ -157,9 +157,13 @@ setup_test_repository () {
 
     mkdir baz
     echo moo > baz/cow
+    echo mooro > baz/cowro
+    chmod 600 baz/cowro
     echo alien > baz/saucer
     mkdir baz/deeper
     echo hi > baz/deeper/ohyeah
+    echo hix > baz/deeper/ohyeahx
+    chmod 755 baz/deeper/ohyeahx
     ln -s nonexistent baz/alink
     mkdir baz/another/
     echo x > baz/another/y
@@ -495,3 +499,7 @@ has_gpgme () {
 libtest_cleanup_gpg () {
     gpg-connect-agent --homedir ${test_tmpdir}/gpghome killagent /bye || true
 }
+
+is_bare_user_only_repo () {
+  grep -q 'mode=bare-user-only' $1/config
+}